Remove a dead case pattern in common.sh (SC2221/SC2222) - #4605
Merged
Merged
Conversation
resolve_template_content() rejects unsafe manifest paths with `/*|*../*|../*)`. The third alternative can never match: `*../*` already matches every string `../*` does, so shellcheck reports SC2221 (this pattern always overrides a later one) and SC2222 (this pattern never matches). Removing it changes nothing: checked against /etc/passwd, ../x, a/../b, ../, x/../../y, .., a/.., ..a, a..b, ./x and templates/x.md -- every input classifies identically before and after. These are warnings, so lint.yml's --severity=error does not see them. This is one of the findings between the current threshold and the tightening CONTRIBUTING.md describes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
jancarlosz039-ui
approved these changes
Sep 16, 2026
mnriem
added
the
triage-can-wait
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The removed pattern is fully subsumed by *../*, so behavior remains unchanged.
Pull request overview
Removes an unreachable Bash case alternative without changing path-validation behavior.
Changes:
- Simplifies the unsafe manifest-path pattern and resolves SC2221/SC2222 warnings.
File summaries
| File | Description |
|---|---|
scripts/bash/common.sh |
Removes the redundant ../* pattern. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Collaborator
|
Thank you! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
resolve_template_content()inscripts/bash/common.shrejects unsafe manifest paths with:The third alternative can never match:
*../*already matches every string../*does. shellcheck reports SC2221 ("this pattern always overrides a later one") and SC2222 ("this pattern never matches") on that line. This PR removes the dead alternative — a one-line change with no behavioural effect.Verified behaviour-identical by classifying these inputs with the old and new pattern:
/etc/passwd,../x,a/../b,../,x/../../y,..,a/..,..a,a..b,./x,templates/x.md. Every one produces the same reject/keep result before and after. After the change, shellcheck reports zero SC2221/SC2222 findings on the file and adds nothing new.These are warnings, so
lint.yml's--severity=errordoes not currently see them. This is one of the findings standing between the current threshold and the tightening described inCONTRIBUTING.md. Related: #1049.Testing
uv run specify --helpuv sync && uv run pytest— 7883 passed, 195 skipped, 1 failed. The failure istests/integrations/test_events.py::TestCommandRunner::test_ps_variant_prefixed_with_powershell_launcher, and it fails identically with this change reverted: it asserts apwsh/powershell -Fileprefix andpwshis not installed on the machine I ran this on (macOS). It is pre-existing and environmental, not caused by this PR; yourmacos-latestrunner haspwshand should be green.CONTRIBUTING.mdfor completeness:Test selection reasoning
scripts/bash/common.shcreate-new-feature.sh,check-prerequisites.sh,setup-plan.sh,setup-tasks.sh,resolve-template.sh(all sourcecommon.sh)resolve_template_content()accepts and rejects exactly the same inputsIf maintainers would still like the manual agent runs, say so and I will do them.
AI Disclosure
This PR was produced by Claude (Anthropic's AI coding agent) operating under my direction: it found the finding while running shellcheck over a project that vendors these scripts, verified the fix, wrote the commit and this description. I reviewed and approved it before it was opened. Any follow-up comments from this account on this PR may also be AI-drafted, and I will keep that disclosed.
🤖 Generated with Claude Code